We are handling both hCaptcha and recaptcha in this file and in the folder in general, so we should rename all files under and along with recaptcha folder to be captcha instead

Fair point. I think we should add this to a new tech debt ticket, along with a few other comments from this PR, to avoid adding more changes into an already large diff.

This deviates from our conventions of utility functions being pure, stateless functions. Typically, anything that involves DOM or Window use is in a Svelte component. But, if something needs to access external systems, we would call it an "effect" (not to be confused with Effect, the library).

With that said, I see that this file already has window use, so we can leave it. But, I do want to stress that we should move away from this as it should be considered an anti-pattern.

Agreed. We can tackle this alongside the stage import in recaptcha.svelte callback file in a new ticket.

Since I was already doing some refactoring, I went ahead and replaced the global window assignments with closures passed directly to renderCaptcha / renderCaptchaInvisible / renderEnterpriseCaptcha. Both reCaptcha and hCaptcha support a programmatic render() API that accepts function refs, so no window lookup needed, no shared-global risk, no core/window.d.ts type.

Don't we need the siteKey in here? or is this changed now?

No, for Enterprise the key is passed to render(element, { sitekey }) or execute(siteKey) at render time, the script just needs to be loaded. The siteKey value comes directly from callback?.getSiteKey().

Update: siteKey isn't needed in the API call for reCaptcha v2 only. Enterprise and v3 does need it though. Fixing now.

This should be fixed.

So we're now creating a distinct ReCaptchaEnterpriseCallback.

Because now in the new "Recaptcha world" there is no significant difference between this for google, this feels like we're diverting our structure away from the actual core Domain (ReCaptcha)

I think maybe what we need to do is align more against HCaptcha vs Google ReCaptcha.

Since GoogleReCaptcha has Enterprise, and others, but they are all under the domain of Enterprise Recaptcha, it can be a bit more resilient to future changes from Google. If they rename, add something, we have just Google based ReCaptcha component that handles that logic, we operate there.

HCaptcha being distinct creates the same separation.

My Concern is that we are creating the separation "Enterprise" vs not, and then if HCaptcha has or adds an Enterprise, are we going to put all Enterprise based logic in the recaptcha-enterprise component?

It feels like the abstraction may be on the wrong thing and should be on the Google vs HCaptcha.

This may help clean up some of the mess I originally created when writing this because now the logic for each is coupled (correctly) to the component that it is.

The issue this does create is in this file, since we check the type, we'd need one more layer of logic which says "Which type" of Recaptcha is this.

I think in this case, this second "check" is worth doing for the separation we can create.

Thoughts?

Fair point. The component split follows AM's callback structure, not our design. AM sends ReCaptchaEnterpriseCallback (different from ReCaptchaCallback) for the Enterprise captcha node, and it has a different API too. One component for both would need duck-typing or internal branching, which can be harder to read and test. hCaptcha only uses ReCaptchaCallback, no Enterprise variant, so it stays in the classic component. If AM adds an hCaptcha Enterprise callback later, it could get its own component or reuse the enterprise one.

If AM is our source of truth for callback components, which it mostly is (one exception aside), a dedicated ReCaptchaEnterpriseCallback makes more sense. That said, I get your point about the abstraction belonging at the ReCaptcha vs. hCaptcha level. It really comes down to following AM's structure or creating our own for captcha callbacks.

I hear this, but i'm not sure that following AM nodes here is the right decision since the real implementation is more specific to the provider. I'm not hard set on this but just my thoughts

I see your point. The existing reCaptchaCallback naming can be specially misleading/confusing since it's used by both reCaptcha and hCaptcha, even though that's the callback name returned by the Captcha node. reCaptchaEnterprise is more consistent with the journey node and its intended use.

I'll spend some time splitting the callback components into hcaptcha.svelte and recaptcha.svelte (classic and enterprise) to see if organizing them by provider makes more sense. Thanks!

I think maybe we should use the actual names of recaptcha here, normal is understandable but I think it may be more clear to use invisible and Not a Robot then we have a third type which is Score based

If we use normal and the world changes to Score based we are out of sync with the standard and we break the api.

Good point. We could rename normal to checkbox or visible if that's clearer. No breaking behavior change, just a string value in the public interface.

Renamed 'normal' to 'visible'. Chose 'visible' over 'checkbox' as it's provider-agnostic and describes the behavior clearly for both Google and hCaptcha. Let me know what you think.

Why not just use the names of the recaptcha instead? "Not a robot" | "invisible" | "score" ?

Because the captcha mode option controls both google reCaptcha and hCaptcha. "Not a robot" is google's checkbox UI copy, not a mode name, and hCaptcha has no equivalent term. 'score' isn't a valid value here either, v3/score-based mode is detected directly from the AM payload (reCaptchaV3: true output) and never needs to be set by the consumer. So the only real choice a consumer makes is how the widget renders: does it show a visible checkbox or execute silently in the background? 'visible' | 'invisible' describes that behavior precisely and stays correct and consistent regardless of the provider.j

Is this still needed now that we have the value woven into the callbackMetadata?

Updated. Passed recaptchaAction through callbackMetadata using the same initOptions pattern as captcha.mode. buildCallbackMetadata now merges recaptchaAction from initializationOptions into initOptions for both captcha callback types. Both components now read callbackMetadata?.initOptions?.recaptchaAction reactively. No store subscription and recaptchaActionStore` removed. Thanks!

can't we change this class at the node level?

This function was detecting the captcha provider based on the class name coming from the captcha callback, but ReCaptchaEnterpriseCallback is Google-only so the hCaptcha branch in the enterprise component was dead code.
Removed detectEnterpriseProvider, the HCaptcha interface, and the hcaptcha render branch. Enterprise always uses grecaptcha.

again i'm concerned that we're relying on this but it's a node level setting if I recall

captchaDivClass is read directly from the callback output (getOutputByName('captchaDivClass')), so the AM node does control it. We need it not just for the css class but to route between hCaptcha and Google. it drives script loading, and the hcaptcha.render() vs grecaptcha.render() branching. It's the only signal the callback exposes that we can use to determine the provider.

np: maybe an object that maps the urls to the type?

Yeah, good call! Replaced the ternary chain with a CAPTCHA_SCRIPT_URLS map key. Thanks!

Since this is a new file, I'd like to move towards the pattern of utilities being pure, stateless functions. Since these manage "effects", like reading from the window object or interacting with the DOM, let's just reorganize/rename this as an effect.

Updated. Split all three files following the WebAuthn pattern. Pure functions remain in *.utilities.ts; DOM/window effects moved to *.effects.ts. Thanks!

I think we should define a type for captcha mode, maybe CaptchaMode. It seems like this value is referenced across lots of files. Defining it as a type will avoid typos and as casting, and provide a single reference across multiple files.

Good call. I added CaptchaMode = 'visible' | 'invisible' type in journey.interfaces.ts. Both callback components now import and use it. This also fixed a TS error where initOptions caused the reactive captchaMode to widen to string, which broke the loadEnterpriseScript mode param. Thanks!

do we want to log out consoles in production code?

No, that was a debug console.error. Removed, thanks.

console in prod code again

Removed. Thank you.